Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

populate imu covariances when converting #375

Merged
merged 8 commits into from
Sep 25, 2023
Merged

populate imu covariances when converting #375

merged 8 commits into from
Sep 25, 2023

Conversation

ejalaa12
Copy link
Contributor

🎉 New feature

Summary

This PR depends on those two other PR:

Once those are merged, this PR will allow forwarding the IMU covariance between ros <--> gz.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

@azeey azeey requested a review from mjcarroll March 20, 2023 18:28
ahcorde
ahcorde requested changes Mar 20, 2023
View reviewed changes
ros_gz_bridge/test/utils/ros_test_msg.cpp Outdated Show resolved Hide resolved
ros_gz_bridge/src/convert/sensor_msgs.cpp Outdated Show resolved Hide resolved
@ejalaa12
fixed unit test
cd666b5 
Signed-off-by: Alaa El Jawad <[email protected]>
@ejalaa12
Copy link
Contributor Author

Again, note that the CI will not pass until the other 2 PRs are merged.

@azeey azeey added the beta label Jul 31, 2023
@mjcarroll
Copy link
Collaborator

Getting CI set up for harmonic here. We are going to need to make sure that we #ifdef the appropriate bits because this message is only supported in harmonic+

@ejalaa12
Copy link
Contributor Author

Do you have an example of this somewhere in the code ? What is the token I should check for ? I couldn't find one.

@mjcarroll
Copy link
Collaborator

Do you have an example of this somewhere in the code ? What is the token I should check for ? I couldn't find one.

I just pushed an implementation to your branch. This isn't something we've really done much, so there isn't documentation on it.

I added a define based on the MAJOR version number of gz-msgs, as covariances are only available in gz-msgs10+

@mjcarroll
Lint
61848c0 
Signed-off-by: Michael Carroll <[email protected]>
@mjcarroll
Undo NavSat changes
15acb0c 
Signed-off-by: Michael Carroll <[email protected]>
@mjcarroll
One index covariance entries
e505346 
Signed-off-by: Michael Carroll <[email protected]>
nkoenig
nkoenig approved these changes Aug 31, 2023
View reviewed changes
Copy link
Contributor

@nkoenig nkoenig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@mjcarroll mjcarroll removed the beta label Aug 31, 2023
@mjcarroll
Copy link
Collaborator

One more test failure here, @ejalaa12 can you take a look?

I think this repo isn't quite as subject to the freeze as there will be no harmonic/rolling binaries generated from here.

@ejalaa12
Copy link
Contributor Author

ejalaa12 commented Sep 25, 2023 •
edited
Loading

@mjcarroll, sorry for the long delay. I just pushed the fix, all checks are passing now :)

@ahcorde ahcorde merged commit 9b0e3a9 into gazebosim:ros2 Sep 25, 2023
4 checks passed
@ejalaa12 ejalaa12 deleted the imu-covariances branch September 25, 2023 20:48
@ejalaa12 ejalaa12 mentioned this pull request Jan 23, 2024
Merged
wittenator pushed a commit to wittenator/ros_gz that referenced this pull request Apr 27, 2024
@ejalaa12 @wittenator
populate imu covariances when converting (gazebosim#375)
d43ebc5 
Signed-off-by: Alaa El Jawad <[email protected]>
wittenator pushed a commit to wittenator/ros_gz that referenced this pull request Apr 27, 2024
@ejalaa12 @wittenator
populate imu covariances when converting (gazebosim#375)
600a377 
Signed-off-by: Alaa El Jawad <[email protected]>
Signed-off-by: wittenator <[email protected]>
ahcorde pushed a commit that referenced this pull request Apr 30, 2024
@wittenator @ejalaa12
populate imu covariances when converting (#375) (#540)
9377674 
Signed-off-by: Alaa El Jawad <[email protected]>
Signed-off-by: wittenator <[email protected]>
Co-authored-by: El Jawad Alaa <[email protected]>
@azeey azeey mentioned this pull request May 6, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahcorde ahcorde ahcorde approved these changes

@nkoenig nkoenig nkoenig approved these changes

@mjcarroll mjcarroll mjcarroll approved these changes

Assignees
No one assigned
Labels
None yet
Projects
Core development
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ejalaa12 @mjcarroll @nkoenig @ahcorde @azeey